Implement locking service #15
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gunarp
commented
May 26, 2022
| @@ -0,0 +1,27 @@ | |||
| @startuml Locking Service Sequence Diagram | |||
There was a problem hiding this comment.
PUML is a diagram generator we use at Microsoft, it's free and is a nice way to create pictures from text.
You can generate the diagram by copying this into the bottom of the plant uml website or downloading a local viewer (there's a vs code extension called PlantUml by Jebbs that has some instructions on that)
Also here's a link to the image
gunarp
commented
May 26, 2022
|
|
||
| # use exponential backoff to try again without flooding network | ||
| while not lock_acquired: | ||
| rospy.loginfo("Couldn't acquire the lock, trying again") |
There was a problem hiding this comment.
Consider changing to a debug message instead, this will pop up whenever a node is waiting on the lock every once in a while.
Anemnox
approved these changes
Jun 15, 2022
Anemnox
requested changes
Jun 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why make this change?
Our current locking system is always-preemptable, which is fine for our use case but isn't ideal as it means the driver has to be pretty careful with the way they launch scripts, as running some scripts over others might get us into an undefined state.
What changed?
Added new classes
LockServerandLockClient, which uses a ros service to implement a mutex which also gives signals to enable control from the UI via the topic/ui/enablementAlso cleaned up the directories in
robot_moduleand modified some of the imports to make them more consistent with other areas of the codebase.How was it tested?
Tested
LockServerclass by runninglock_service_server.pyand sending messages usingrosservice.Also created a test file showing how to use
LockClient,example_lock_client.py, and verified that it can be executed (assuming nothing's holding the lock already).Also double checked that
test.pyis still runnable after making changes to the imports.